-
Notifications
You must be signed in to change notification settings - Fork 534
Suggest not requesting one-by-one in rule 2.1 #435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
README.md
Outdated
@@ -131,7 +131,7 @@ public interface Subscriber<T> { | |||
| ID | Rule | | |||
| ------------------------- | ------------------------------------------------------------------------------------------------------ | | |||
| <a name="2.1">1</a> | A `Subscriber` MUST signal demand via `Subscription.request(long n)` to receive `onNext` signals. | | |||
| [:bulb:](#2.1 "2.1 explained") | *The intent of this rule is to establish that it is the responsibility of the Subscriber to signal when, and how many, elements it is able and willing to receive.* | | |||
| [:bulb:](#2.1 "2.1 explained") | *The intent of this rule is to establish that it is the responsibility of the Subscriber to signal when, and how many, elements it is able and willing to receive. It is RECOMMENDED to request as many elements as a Subscriber is prepared to receive (e.g. has preallocated buffer space for), rather than requesting "one by one".* | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, the buffer space part only applies for asynchronous Subscribers.
Would this be more general?
For performance reasons it is RECOMMENDED to request as many elements as a Subscriber is currently able to receive, as requesting elements one at a time will be the slowest possible option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I don't like starting with "for performance reasons", always feels hand wavy right away, but good rewording; how about:
It is RECOMMENDED to request as many elements as a Subscriber is currently able to receive, as the most conservative strategy to signal demand by requesting elements one-at-a-time is the inherently inefficient.
?
Or maybe: It is RECOMMENDED that Subscribers request as many elements as can be processed without loss, within their resource limits. Requesting only one item at a time results in an inherently inefficient "stop-and-wait" protocol. Maybe we could add a link to something comparing stop-and-wait to windowing. All the ones that come to mind assume people know about TCP protocols etc, which might not be helpful. |
I like that wording a lot @DougLea, thanks! Would it be fine if I use your proposed wording in this PR? |
Sounds find to me. The "without loss" phrasing is not strictly necessary I think (I mean it could be a dropping / filtering operation after all) |
@ktoso Right, and some operators do not want too trigger much look-ahead (a headOption / findFirst). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
lgtm |
@ktoso Rebase and resolve conflict and I'll merge. :) |
resolved |
Perfect, thanks @ktoso! |
Follow up from #433 (comment)
Improves intent wording to explain that implementations should try to request as many elements as it makes sense, and not only request(1) as it defeats the pipelining potential of reactive streams.